fix(security): cap /api/file + MCP read_file at 5 MiB (RAN-9)#67
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7be4f6a670
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…N-9)
Without a size guard, /api/file and the read_file MCP tool loaded entire
files into the JVM heap before the optional line slice was applied. A
multi-GB file in an indexed codebase trivially OOM'd the serving process,
giving anyone with HTTP access a DoS lever against the read-only API and
MCP endpoints.
Adds a configurable cap (default 5 MiB, key: serving.max_file_bytes) and
routes both entry points through a single SafeFileReader:
- No line range → check on-disk size first; reject before reading.
- With a line range → stream via BufferedReader, apply start/end filter,
and track accumulated UTF-8 byte count against the cap.
REST returns HTTP 413 (CONTENT_TOO_LARGE); MCP returns the usual JSON
{\"error\": ...} payload. Both emit a message shaped like
\"File exceeds max size: N bytes (max M bytes)\".
Config is plumbed end-to-end through the unified config stack:
ServingConfig.maxFileBytes, ConfigDefaults.builtIn() (5 MiB),
ConfigMerger, EnvVarOverlay (CODEIQ_SERVING_MAXFILEBYTES), snake_case
YAML key with camelCase alias, UnifiedConfigAdapter → legacy
CodeIqConfig.maxFileBytes (with a >=1 clamp in the setter).
Tests added:
- SafeFileReaderTest: whole-file reject, line-range streaming, line-range
reject mid-stream, negative startLine clamping.
- GraphControllerTest: 413 on oversize whole-file read, 200 on narrow
line range when the whole file exceeds cap.
- McpToolsTest: \"exceeds max size\" error for oversize read, line-range
passthrough, line-range reject.
- UnifiedConfigAdapterTest: explicit maxFileBytes overrides default;
absent value falls back to CodeIqConfig default (5 MiB).
- ConfigResolverTest / ConfigValidatorTest / ConfigExplainSubcommandTest:
updated for the new ServingConfig record shape.
Default documented in docs/codeiq.yml.example.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
aksOps
force-pushed
the
fix/ran-9-max-file-bytes-cap
branch
from
7be4f6a to
f91c37e
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/api/fileand theread_fileMCP tool previously loaded entire files into JVM heap viaFiles.readString(...)before applying any line slice. A multi-GB file in an indexed codebase would OOM the serving process → trivial DoS against the read-only API/MCP surface.serving.max_file_bytes, default 5 MiB) enforced for both entry points via a singleSafeFileReader:BufferedReader, applystart/endfilter, and track accumulated UTF-8 byte count against the cap.CONTENT_TOO_LARGE); MCP returns the usual JSON{\"error\": ...}payload. Error message:File exceeds max size: N bytes (max M bytes).ServingConfig.maxFileBytes→ConfigDefaults.builtIn()(5 MiB) →ConfigMerger→EnvVarOverlay(CODEIQ_SERVING_MAXFILEBYTES) → snake_case YAML with camelCase alias →UnifiedConfigAdapter→ legacyCodeIqConfig.maxFileBytes(setter clamps to>=1).docs/codeiq.yml.example.Closes RAN-9.
Test plan
SafeFileReaderTest— whole-file reject, line-range streaming, line-range reject mid-stream, negativestartLineclamping.GraphControllerTest— 413 on oversize whole-file read; 200 on narrow line range when whole file > cap.McpToolsTest—\"exceeds max size\"error for oversize read; line-range passthrough; line-range reject.UnifiedConfigAdapterTest— explicitmaxFileBytesoverrides default; absent value falls back toCodeIqConfigdefault (5 MiB).ConfigResolverTest/ConfigValidatorTest/ConfigExplainSubcommandTest— updated for the newServingConfigrecord shape.Tests run: 231, Failures: 0, Errors: 0, Skipped: 0.Acceptance (from RAN-9)
codeiq.yml.example.